Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(rocky): add multi-arch support #321

Merged
merged 22 commits into from
Jul 3, 2023
Merged

fix(rocky): add multi-arch support #321

merged 22 commits into from
Jul 3, 2023

Conversation

DmitriyLewen
Copy link
Contributor

@DmitriyLewen DmitriyLewen commented Jun 21, 2023 •
edited
Loading

Description

Rocky Linux supports aarch64.
These are cases when fixed version for x86_64 != aarch64 or there is no fixed version for x86_64 architecture.

We need to add advisories for aarch64 and separate architectures.
I add Advisory.FixedVersions struct for this case.
It will be used to separate architectures, versions, vendorIDs.
Advisory.FixedVersion field is still used for backwards compatibility.

db size before - 361.0 Mb
db size after - 361.1Mb

buckets struct before:
image

buckets after after:
image

Related Issues

@DmitriyLewen DmitriyLewen self-assigned this Jun 21, 2023
@DmitriyLewen DmitriyLewen marked this pull request as ready for review June 21, 2023 06:49
knqyf263
knqyf263 reviewed Jun 21, 2023
View reviewed changes
pkg/types/types.go Outdated
@@ -116,6 +116,9 @@ type Advisory struct {
FixedVersion string `json:",omitempty"`
AffectedVersion string `json:",omitempty"` // Only for Arch Linux

// Advisory can contain different fixed versions for different architectures
FixedVersions FixedVersions `json:",omitempty"`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Entries is better to get aligned with Red Hat. I'm not sure if it is a good name, though.

trivy-db/pkg/vulnsrc/redhat-oval/types.go

Lines 160 to 162 in c76cb31

type Advisory struct {
Entries []Entry `json:",omitempty"`
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about it and couldn't find good name.
But you are right. We use Enties in RedHat, let's also use Entries here.

Done in 14b1e69

pkg/vulnsrc/rocky/rocky.go Outdated
// if the advisory for this package and CVE have been kept - just add the new architecture
if adv, ok := input.Advisories[pkg.Name]; ok {
// don't include duplicates
if !adv.FixedVersions.IsDuplicate(fixedVersion) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess slices.Contains works.

Suggested change
if !adv.FixedVersions.IsDuplicate(fixedVersion) {
if !slices.Contains(adv.FixedVersions, fixedVersion) {

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea!
Changed in 14b1e69

@knqyf263
Copy link
Collaborator

knqyf263 commented Jun 21, 2023 •
edited
Loading

During reviewing your PR, I came up with another idea. What if using vendor id as the bucket key instead of CVE-ID?

trivy-db/pkg/types/types.go

Line 98 in d500e5f

VulnerabilityID string `json:",omitempty"` // CVE-ID or vendor ID

Then, we add CVE-IDs into VendorIDs. The field name doesn't fit, though. We will probably rename it to Aliases and use a different name for JSON for compatibility.

// Aliases represents the list of vulnerability identifiers that serve as aliases for the main vulnerability ID.
// e.g. CVE-ID, RHSA-ID and DSA-ID
// 
// The JSON key must be "VendorIDs" to keep backward compatibility.
// TODO: rename "VendorIDs" to "Aliases" when we bump the schema version of Trivy DB.
Aliases       []string `json:"VendorIDs,omitempty"`

The bucket will be like

- rocky 9
    - 389-ds-base
        - RLSA-2022:8162
            - Arches: []string{"aarch64", "x86_64"}
              FixedVesion: "2.1.3-4.el9_1",
              Aliases: []string{"CVE-2022-0918"}
        - RLSA-2022:8163 (dummy advisory for explanation)
            - Arches: []string{"aarch64"}
              FixedVesion: "2.1.3-100.el9_1",
              Aliases: []string{"CVE-2022-0919", "CVE-2022-0920"}
        - RLSA-2022:8164 (dummy advisory for explanation)
            - Arches: []string{"x86_64"}
              FixedVesion: "2.1.3-200.el9_1",
              Aliases: []string{"CVE-2022-0919", "CVE-2022-0920"}

We already did the similar thing in Red Hat.

@DmitriyLewen
Copy link
Contributor Author

I see 1 case why we can't do that ( but i didn't see example for Rocky linux, but OracleLinux contains this case):
There is Vendor ID with different fixed versions for different architectures.
e.g. https://linux.oracle.com/errata/ELSA-2022-4803.html :
Oracle Linux 7 (aarch64) | rsyslog-8.24.0-57.0.4.el7_9.3.src.rpm
Oracle Linux 7 (x86_64) | rsyslog-8.24.0-57.0.1.el7_9.3.src.rpm

Also i have 1 thought for new struct:
We can add Flavors for OracleLinux - #221

@DmitriyLewen DmitriyLewen mentioned this pull request Jun 22, 2023
Merged
8 tasks
@knqyf263 knqyf263 force-pushed the feat/fixed-versions-with-arch-struct branch from 2a66c5b to 6915173 Compare June 29, 2023 13:47
@knqyf263 knqyf263 changed the title feat(rocky): add support of Advisory.FixedVersions struct fix(rocky): add multi-arch support Jul 3, 2023
@knqyf263 knqyf263 merged commit 9f4852a into aquasecurity:main Jul 3, 2023
@DmitriyLewen DmitriyLewen mentioned this pull request Jul 3, 2023
Merged
@DmitriyLewen DmitriyLewen deleted the feat/fixed-versions-with-arch-struct branch July 12, 2023 03:20
@DmitriyLewen DmitriyLewen mentioned this pull request Jul 12, 2023
Open
@Moniseeta Moniseeta mentioned this pull request Nov 2, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@knqyf263 knqyf263 knqyf263 approved these changes

Assignees

@DmitriyLewen DmitriyLewen

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@DmitriyLewen @knqyf263